-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
updating to macos-14 #1486
updating to macos-14 #1486
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with it, but to check of this will work, since I thought you need npx there....
@@ -272,7 +272,7 @@ jobs: | |||
- name: Verify zap-cli exists in Windows arm64 .zip package | |||
if: startsWith(matrix.os, 'macos') | |||
run: | | |||
output=$(./node_modules/7zip-bin/mac/x64/7za l ./dist/zap-win-arm64.zip) | |||
output=$(7za l ./dist/zap-win-arm64.zip) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that's what npx
is for? Shouldn't you do npx 7za
? I am confused why are you changing this path.
It is true, that running ./node_modules
like it was is wrong, but npx
should do that for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/actions/runner-images/blob/main/images/macos/macos-14-Readme.md
7zip comes pre-installed as a utility in the macos-14 environment. I could change it to use the node module though.
f593771
to
894a10c
Compare
also using 7zip pre-installed library instead of using node module